Support struct for Java #259

honnix · 2023-10-08T17:00:48Z

Read then delete this section

TL;DR

Support nested auto-value as input parameters.

Type

Bug Fix
Feature
Plugin

Are all requirements met?

Complete description

This is to add support for things like:

@AutoValue
class Foo {
  public abstract SdkBindingData<String> foo();

  public abstract SdkBindingData<Bar> bar();
}

@AutoValue
class Bar {
  // note that this is not an SdkBindingData because it doesn't make sense since the whole class is wrapped as a promise
  // this also matches the direction of the existing but disabled IT
  public abstract String bar();
}

Scala layer is not supported yet because it is completely different mechanism.

Tracking Issue

_NA

Follow-up issue

NA

honnix · 2023-10-09T08:57:26Z

flytekit-jackson/src/main/java/org/flyte/flytekit/jackson/deserializers/StructDeserializer.java


-public class LiteralStructDeserializer extends StdDeserializer<Literal> {


It was confusing to use Literal as the target but expect a Struct.

honnix · 2023-10-09T08:59:54Z

flytekit-jackson/src/main/java/org/flyte/flytekit/jackson/serializers/GenericSerializer.java

-  interface WriteGenericFunction {
-    void write(JsonGenerator gen, Struct.Value value) throws IOException;
+    gen.writeFieldName(VALUE);
+    new StructSerializer().serialize(value.scalar().generic(), gen, serializerProvider);


The idea is to serialize a tree that can be directly deserialized into an auto-value object, which is exactly what StructSerializer does.

honnix · 2023-10-09T09:29:12Z

...-jackson/src/main/java/org/flyte/flytekit/jackson/deserializers/LiteralMapDeserializers.java

-import org.flyte.api.v1.LiteralType;
-import org.flyte.flytekit.jackson.JacksonLiteralMap;
-
-public class LiteralMapDeserializers extends Deserializers.Base {


This is no longer be used since we don't do a dynamic lookup anymore.

honnix · 2023-10-09T09:30:08Z

...ckson/src/main/java/org/flyte/flytekit/jackson/deserializers/SdkBindingDataDeserializer.java

+
+  @Override
+  public JsonDeserializer<?> createContextual(DeserializationContext ctxt, BeanProperty property) {
+    return new SdkBindingDataDeserializer(property.getType().containedType(0));


This is a funny way to get bean property context.

honnix · 2023-10-09T18:46:38Z

I can no longer run IT locally because testcontainers generates a warning Unable to mount a file from test host into a running container. This may be a misconfiguration or limitation of your Docker environment. Some features might not work. so I cannot mount things properly during IT.

honnix · 2023-10-09T20:06:23Z

I can no longer run IT locally because testcontainers generates a warning Unable to mount a file from test host into a running container. This may be a misconfiguration or limitation of your Docker environment. Some features might not work. so I cannot mount things properly during IT.

Trying a fix in #261.

honnix · 2023-10-09T21:17:01Z

.scalafmt.conf

@@ -1,3 +1,3 @@
-version=2.5.2


This does not support scala212source3.

honnix · 2023-10-14T20:03:41Z

flytekit-jackson/src/test/java/org/flyte/flytekit/jackson/JacksonSdkTypeTest.java

-  }
-
-  @AutoValue
-  public abstract static class StructInput {


I only discovered these classes in a very late stage so I did not bother adapting test cases to use these.

Signed-off-by: Hongxin Liang <[email protected]>

andresgomezfrr

LGTM 🚀

honnix · 2023-10-17T08:26:25Z

This branch targets blob branch. Now #258 is a bit hard to review.

Signed-off-by: Hongxin Liang <[email protected]>

andresgomezfrr · 2023-10-17T08:42:51Z

This branch targets blob branch. Now #258 is a bit hard to review.

oh! I didn't notice that, don't worry, I will continue with the blob's PR

Signed-off-by: Hongxin Liang <[email protected]>

* Bring back Blob support Signed-off-by: Hongxin Liang <[email protected]> * Remove BlobTypeDescription Signed-off-by: Hongxin Liang <[email protected]> * Add blob back Signed-off-by: Hongxin Liang <[email protected]> * Blob in list and map Signed-off-by: Hongxin Liang <[email protected]> * Clean up Signed-off-by: Hongxin Liang <[email protected]> * Support struct (#259) Signed-off-by: Hongxin Liang <[email protected]> * Support struct in Scala layer (#262) Signed-off-by: Hongxin Liang <[email protected]> --------- Signed-off-by: Hongxin Liang <[email protected]>

honnix force-pushed the struct branch 3 times, most recently from 1a85668 to 9f36a57 Compare October 8, 2023 20:34

honnix force-pushed the blob branch from 3280083 to 4d261c9 Compare October 8, 2023 21:15

honnix force-pushed the struct branch from 9f36a57 to fd3d2c6 Compare October 8, 2023 21:15

honnix commented Oct 9, 2023

View reviewed changes

honnix changed the title ~~Support struct~~ Support struct for Java Oct 9, 2023

honnix marked this pull request as ready for review October 9, 2023 09:23

honnix commented Oct 9, 2023

View reviewed changes

honnix force-pushed the blob branch 2 times, most recently from 8fb94fb to ff12243 Compare October 9, 2023 18:17

honnix force-pushed the struct branch 2 times, most recently from e40e5c2 to e903e57 Compare October 9, 2023 18:45

honnix force-pushed the struct branch from e903e57 to cfd81b4 Compare October 9, 2023 19:34

honnix force-pushed the struct branch from cfd81b4 to 3c7064a Compare October 9, 2023 20:07

honnix force-pushed the blob branch 2 times, most recently from 6ea206b to 8284ee3 Compare October 9, 2023 21:09

honnix force-pushed the struct branch 2 times, most recently from 724849c to 7032727 Compare October 9, 2023 21:15

honnix commented Oct 9, 2023

View reviewed changes

.scalafmt.conf

@@ -1,3 +1,3 @@

version=2.5.2

Copy link

Member Author

honnix Oct 9, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This does not support scala212source3.

honnix force-pushed the struct branch 3 times, most recently from 6742be4 to 8a84063 Compare October 10, 2023 11:10

honnix force-pushed the blob branch from cc493fa to f6e7942 Compare October 10, 2023 11:10

honnix force-pushed the struct branch from 8a84063 to 4de3436 Compare October 11, 2023 11:26

honnix mentioned this pull request Oct 11, 2023

Struct scala #262

Merged

8 tasks

honnix force-pushed the struct branch 3 times, most recently from b207975 to 768b8bc Compare October 14, 2023 19:57

honnix commented Oct 14, 2023

View reviewed changes

honnix force-pushed the struct branch from 768b8bc to 38d3ffa Compare October 15, 2023 08:14

Support struct

2775c03

Signed-off-by: Hongxin Liang <[email protected]>

honnix force-pushed the struct branch from 38d3ffa to 2775c03 Compare October 15, 2023 08:51

andresgomezfrr approved these changes Oct 17, 2023

View reviewed changes

andresgomezfrr merged commit 78aea79 into blob Oct 17, 2023
3 checks passed

andresgomezfrr deleted the struct branch October 17, 2023 08:09

honnix added a commit that referenced this pull request Oct 17, 2023

Support struct (#259)

de26c25

Signed-off-by: Hongxin Liang <[email protected]>

honnix mentioned this pull request Oct 17, 2023

Blob and struct support #258

Merged

8 tasks

honnix added a commit that referenced this pull request Oct 17, 2023

Support struct (#259)

865f81b

Signed-off-by: Hongxin Liang <[email protected]>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support struct for Java #259

Support struct for Java #259

honnix commented Oct 8, 2023 •

edited

Loading

honnix Oct 9, 2023

honnix Oct 9, 2023 •

edited

Loading

honnix Oct 9, 2023

honnix Oct 9, 2023

honnix commented Oct 9, 2023

honnix commented Oct 9, 2023

honnix Oct 9, 2023

honnix Oct 14, 2023

andresgomezfrr left a comment

honnix commented Oct 17, 2023

andresgomezfrr commented Oct 17, 2023


		public class LiteralStructDeserializer extends StdDeserializer<Literal> {

Support struct for Java #259

Support struct for Java #259

Conversation

honnix commented Oct 8, 2023 • edited Loading

Read then delete this section

TL;DR

Type

Are all requirements met?

Complete description

Tracking Issue

Follow-up issue

honnix Oct 9, 2023

Choose a reason for hiding this comment

honnix Oct 9, 2023 • edited Loading

Choose a reason for hiding this comment

honnix Oct 9, 2023

Choose a reason for hiding this comment

honnix Oct 9, 2023

Choose a reason for hiding this comment

honnix commented Oct 9, 2023

honnix commented Oct 9, 2023

honnix Oct 9, 2023

Choose a reason for hiding this comment

honnix Oct 14, 2023

Choose a reason for hiding this comment

andresgomezfrr left a comment

Choose a reason for hiding this comment

honnix commented Oct 17, 2023

andresgomezfrr commented Oct 17, 2023

honnix commented Oct 8, 2023 •

edited

Loading

honnix Oct 9, 2023 •

edited

Loading